[node] - Install pnpm as non-root user to prevent root-owned npm cache#1625
[node] - Install pnpm as non-root user to prevent root-owned npm cache#1625brianhelba wants to merge 2 commits intodevcontainers:mainfrom
Conversation
Kaniska244
left a comment
Kaniska244
left a comment
There was a problem hiding this comment.
Hi @brianhelba
Thank you for the contribution. Would you kindly bump the node feature version.
Currently, the pnpm installation block runs `npm install -g pnpm` in
a bare subshell as root, unlike every other npm/nvm operation in the
script which uses `su ${USERNAME}`. This causes the npm cache directory
to be created owned by `root:root`, leading to `EACCES` errors for the
non-root user on subsequent npm operations.
This is particularly reproducible on macOS with Rosetta 2 emulation,
where the cache directory may not already exist from prior steps.
Note, the explicit setting of proxy env vars (`http_proxy`, `https_proxy`,
`no_proxy`) is likely a workaround for npm/cli#6835.
No other commands in this script use that workaround anymore, so this change
uses the same `su` syntax as all other commands.
|
@Kaniska244 Done. This is simply a bugfix and doesn't add or remove features, so I bumped the patch version. |
Kaniska244
left a comment
Kaniska244
left a comment
There was a problem hiding this comment.
To be further reviewed by maintainers.
Hi @brianhelba Would you please accept the above license agreement. |
|
@Kaniska244 Sorry, I'm awaiting approval from my employer. I'm hoping this will be quick. |
|
@microsoft-github-policy-service agree company="Kitware, Inc." |
|
@Kaniska244 All done. |
| if bash -c ". '${NVM_DIR}/nvm.sh' && type npm >/dev/null 2>&1"; then | ||
| ( | ||
| . "${NVM_DIR}/nvm.sh" | ||
| [ ! -z "$http_proxy" ] && npm set proxy="$http_proxy" |
There was a problem hiding this comment.
Others may need these proxies; could they be added back?
| [ ! -z "$no_proxy" ] && npm set noproxy="$no_proxy" | ||
| npm install -g pnpm@$PNPM_VERSION --force | ||
| ) | ||
| su ${USERNAME} -c "umask 0002 && . '${NVM_DIR}/nvm.sh' && npm install -g pnpm@${PNPM_VERSION} --force" |
There was a problem hiding this comment.
Are there any implications for switching to user-based vs root based? There may have been assumptions made by others based on this being root-based. If it could cause issues, we may want to consider adding an option and defaulting to the original approach.
abdurriq
left a comment
abdurriq
left a comment
There was a problem hiding this comment.
Thank you for your contribution! I've left a couple comments which I would appreciate if you could review and address.
3 participants
Currently, the pnpm installation block runs
npm install -g pnpmin a bare subshell as root, unlike every other npm/nvm operation in the script which usessu ${USERNAME}. This causes the npm cache directory to be created owned byroot:root, leading toEACCESerrors for the non-root user on subsequent npm operations.This is particularly reproducible on macOS with Rosetta 2 emulation, where the cache directory may not already exist from prior steps.
Note, the explicit setting of proxy env vars (
http_proxy,https_proxy,no_proxy) is likely a workaround for npm/cli#6835. No other commands in this script use that workaround anymore, so this change uses the samesusyntax as all other commands.